-
Notifications
You must be signed in to change notification settings - Fork 222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
reload region cache when store is resolved from invalid status (#843) #846
Conversation
cannot build. |
Hold this PR because it may spawns too many goroutines. |
See the discussions in #843.
|
Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: Smityz <[email protected]> Co-authored-by: disksing <[email protected]> Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: disksing <[email protected]> Signed-off-by: you06 <[email protected]>
d8bd792
to
3b066b5
Compare
mockRequestLiveness atomic.Value | ||
} | ||
|
||
regionsNeedReload struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using a channel so Mutex
could be saved and operations on the RegionCache
are already synchronized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A channel is bounded, if it's full when trying scheduling a region to it, it'll wait(maybe the asyncCheckAndResolveLoop
is doing something and cannot pull the channel immediately).
internal/locate/region_cache.go
Outdated
for regionID := range c.regionsNeedReload.toReload { | ||
c.reloadRegion(regionID) | ||
delete(c.regionsNeedReload.toReload, regionID) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look like those codes should be put after line #498 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delay the reload to next loop which avoids some errors and backoffs, see the comment of Line491 to Line494
Co-authored-by: crazycs <[email protected]> Signed-off-by: you06 <[email protected]>
Signed-off-by: you06 <[email protected]>
3d629f8
to
98e21b3
Compare
Signed-off-by: you06 <[email protected]>
@@ -1300,7 +1359,7 @@ func (c *RegionCache) removeVersionFromCache(oldVer RegionVerID, regionID uint64 | |||
|
|||
// insertRegionToCache tries to insert the Region to cache. | |||
// It should be protected by c.mu.Lock(). | |||
func (c *RegionCache) insertRegionToCache(cachedRegion *Region) { | |||
func (c *RegionCache) insertRegionToCache(cachedRegion *Region, invalidateOldRegion bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to comment about the new parameter.
What's wrong with golangci-lint 🤔️
|
@crazycs520 @ekexium @zyguan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Cherry-pick #843 to tidb-6.5.